Skip to content

feat(eve): Redis-backed memory integration for eve's native memory slots - #33

Merged
CahidArda merged 34 commits into
mainfrom
feat/eve-redis-memory
Sep 4, 2026
Merged

feat(eve): Redis-backed memory integration for eve's native memory slots#33
CahidArda merged 34 commits into
mainfrom
feat/eve-redis-memory

Conversation

@upstash-tag

@upstash-tag upstash-tag Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Adds a new subpath, @upstash/agentkit-eve/memory, with two complementary Redis-backed pieces for eve's native memory-slot feature: redisDocuments() (a MemoryDocumentBackend over @upstash/redis, using an atomic Lua EVAL compare-and-set for optimistic-concurrency writes since the REST client has no WATCH/MULTI) and redisMemory() (a full MemoryProvider wrapping the existing AgentMemory for automatic recall/capture on eve's turn and compaction lifecycle hooks, plus BM25 ranked/fuzzy recall). Both are additive — the existing tool-based memory in packages/sdk, packages/eve/src/memory.ts, packages/ai-sdk and eve-extension is untouched and remains ai-sdk's only memory path.

Follow-up: added 12 tests closing a coverage gap flagged in review — the existing suite only asserted that a save happened, never that recall actually invoked AgentMemory.recall at the right lifecycle hooks, nor that captured memories were actually persisted to and readable back from Redis. New tests spy/verify the recall/capture call sites directly, assert real Redis state after a capture (keys + json.get content), and prove a full capture→Redis→recall round trip; the eve-demo eval gained two persistence-verifying gates that scan Redis for a per-run nonce. Mutation-tested to confirm they fail without the underlying wiring.


Built by upstash-tag · mission 625458a1-b6a5-4562-be3c-e3c0419fe5c2 · feat/eve-redis-memorymain · $42.20

upstash-tag Bot added a commit that referenced this pull request Sep 2, 2026
…sistence

Review on #33: "There are tests checking the memory tool of profile, but
there's nothing checking the recall memory. Nothing that checks whether the
recall methods are called and things are saved to redis."

Fair. The live suite only ever drove `turn.started`/`turn.completed`, and it
asserted on the rendered block — so a provider that recalled from an
in-process cache, queried the wrong index, or never wired the compaction
hooks would still have passed, and nothing ever read a memory document back
out of Redis.

Recall/capture invocation — a new offline suite, deterministic (no network,
no BM25, no indexing lag). It spies `AgentMemory.prototype.recall`/`add`,
and where it doesn't spy it runs the real AgentMemory over a scripted client
that records every index query and json.set. It pins down:

  - recall["turn.started"] AND recall["compaction.completed"] each delegate
    to AgentMemory.recall exactly once, with the sanitized locked scope, the
    configured topK/minScore, and the caller's own words as the query;
  - the call reaches Redis as `search.index({name:"agentkit_memory"})` +
    `query({filter:{userId:{$eq},text:{$smart}},limit})` — tenant-scoped and
    fuzzy, on the shared index rather than a slot-private one;
  - the rows the index returns are what the model sees ("<id>: <text>");
  - a replayed operationId re-queries the index ZERO times (the cache has to
    short-circuit the search, not just the formatting), while a fresh
    operationId queries again and sees new state;
  - a text query that matches nothing falls back to a filter-only query;
  - capture["turn.completed"] AND capture["compaction.requested"] each add
    every user message (never the assistant's) through AgentMemory.add with
    a content-hash id, then wait for indexing;
  - the write lands as one json.set per memory under the scope's prefix.

Redis persistence — three new live tests. One asserts real Redis state after
a capture: `keys` returns exactly the content-addressed keys (derived in the
test from stableHash, not hardcoded) and `json.get` equals
{text,userId,createdAt}. One takes the id and text back OUT of Redis and
asserts the recalled block contains that exact "<id>: <text>", closing
capture -> Redis -> recall. One does the same round trip through
compaction.requested -> compaction.completed. Isolated scopes now go through
a `newScope()` helper that registers them for cleanup; two were leaking keys.

eve-demo's eval goes 7 -> 9 gates: the captured fact carries a per-run nonce,
`Redis.fromEnv()` inside the eval scans agentkit:memory:* and asserts the
stored document contains it, and the recalled block must contain it too — so
persistence is proven through eve's own runtime and cannot pass on a document
an earlier run left behind.

All of it mutation-checked: dropping the two compaction hooks and the
memory.add call turns 10 tests red; `capture: false` on the demo slot turns
3 eval gates red, including the new persistence one.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
upstash-tag Bot and others added 23 commits September 3, 2026 12:42
Adds @upstash/agentkit-eve/memory with two complementary pieces:
- redisDocuments(): a MemoryDocumentBackend backed by @upstash/redis, filling
  eve's documented fileMemory() gap outside Vercel. Optimistic concurrency
  (MemoryDocumentConflictError) via an atomic Lua eval compare-and-set, since
  @upstash/redis is REST-only (no WATCH/MULTI).
- redisMemory(): a full MemoryProvider wrapping the existing AgentMemory (BM25
  ranked/fuzzy recall), giving eve automatic recall/capture at its turn and
  compaction lifecycle hooks, plus <slot>__save_memory/forget_memory tools.

Existing tool-based memory (sdk AgentMemory, eve/src/memory.ts, ai-sdk memory,
eve-extension recall/save tools) is untouched and stays the only memory path
for ai-sdk. Wires an eve-demo example (agent/memory/*.ts) with a mocked-model
eval exercising both pieces end to end, added to CI.
… just wrote

CI run 33600941304 red on one assertion:

  packages/eve/src/eve-memory.test.ts:178
  redisDocuments() — MemoryDocumentBackend (live Redis)
    > creates with expectedVersion null, then round-trips through read
  AssertionError: expected null to deeply equal { content: 'first', …(1) }

`write()` had returned normally, so the Lua CAS had run and the HSET had
executed; the HMGET issued immediately after it saw nothing, and every later
read of the same key in the same file succeeded. That is a read overtaking
replication.

Upstash serves read-your-writes with an `upstash-sync-token` header, and
`@upstash/redis@1.38.0` sends it one request late: `HttpClient.request()`
builds `requestHeaders` from `this.headers` and only afterwards copies
`this.upstashSyncToken` into `this.headers`, so every request carries the
token from one response ago. The read straight after a write therefore
travels with a token that pre-dates the write and a replica is free to
answer from behind. It is a race — the replica is normally current within
the round trip — which is why ~15 other write-then-read pairs in the same
file passed and a single-region dev database never reproduced it.

A false "absent" is the one answer that actually costs something: eve's
`fileMemory()` reacts by starting a fresh document and writing it with
`expectedVersion: null`, which conflicts and retries. So the backend now
keeps a bounded FIFO set of scope keys it has written and confirms an
"absent" answer for one of them with up to two re-reads — any extra request
flushes the correct sync token, so the retry is the request that carries it.
Keys this instance never wrote still resolve to `null` on the first read, so
the common "no document yet" path is unchanged at one round trip.

Reproduced deterministically with a scripted lagging client rather than
waiting on the race: the two new offline tests fail against the previous
`read()` with the exact CI message and pass with this one. The `ttlSeconds`
assertion now polls, since `redis.ttl` is a raw metadata read that `read()`
cannot cover. CLAUDE.md records the sync-token behaviour under Testing — it
is a latent flake for every live-Redis suite in the repo, not just this one.

No design decision, export or file from the original change is altered.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`redis.exists` right after `forget_memory`'s `del` is the same raw
read-after-write that a lagging replica can answer stale — the inverse of the
case the previous commit fixed, and one `read()` cannot cover because it is a
raw metadata read. Poll it like the `ttlSeconds` assertion.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sistence

Review on #33: "There are tests checking the memory tool of profile, but
there's nothing checking the recall memory. Nothing that checks whether the
recall methods are called and things are saved to redis."

Fair. The live suite only ever drove `turn.started`/`turn.completed`, and it
asserted on the rendered block — so a provider that recalled from an
in-process cache, queried the wrong index, or never wired the compaction
hooks would still have passed, and nothing ever read a memory document back
out of Redis.

Recall/capture invocation — a new offline suite, deterministic (no network,
no BM25, no indexing lag). It spies `AgentMemory.prototype.recall`/`add`,
and where it doesn't spy it runs the real AgentMemory over a scripted client
that records every index query and json.set. It pins down:

  - recall["turn.started"] AND recall["compaction.completed"] each delegate
    to AgentMemory.recall exactly once, with the sanitized locked scope, the
    configured topK/minScore, and the caller's own words as the query;
  - the call reaches Redis as `search.index({name:"agentkit_memory"})` +
    `query({filter:{userId:{$eq},text:{$smart}},limit})` — tenant-scoped and
    fuzzy, on the shared index rather than a slot-private one;
  - the rows the index returns are what the model sees ("<id>: <text>");
  - a replayed operationId re-queries the index ZERO times (the cache has to
    short-circuit the search, not just the formatting), while a fresh
    operationId queries again and sees new state;
  - a text query that matches nothing falls back to a filter-only query;
  - capture["turn.completed"] AND capture["compaction.requested"] each add
    every user message (never the assistant's) through AgentMemory.add with
    a content-hash id, then wait for indexing;
  - the write lands as one json.set per memory under the scope's prefix.

Redis persistence — three new live tests. One asserts real Redis state after
a capture: `keys` returns exactly the content-addressed keys (derived in the
test from stableHash, not hardcoded) and `json.get` equals
{text,userId,createdAt}. One takes the id and text back OUT of Redis and
asserts the recalled block contains that exact "<id>: <text>", closing
capture -> Redis -> recall. One does the same round trip through
compaction.requested -> compaction.completed. Isolated scopes now go through
a `newScope()` helper that registers them for cleanup; two were leaking keys.

eve-demo's eval goes 7 -> 9 gates: the captured fact carries a per-run nonce,
`Redis.fromEnv()` inside the eval scans agentkit:memory:* and asserts the
stored document contains it, and the recalled block must contain it too — so
persistence is proven through eve's own runtime and cannot pass on a document
an earlier run left behind.

All of it mutation-checked: dropping the two compaction hooks and the
memory.add call turns 10 tests red; `capture: false` on the demo slot turns
3 eval gates red, including the new persistence one.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`add()` accepts a `conversationId` and `recall()` returns it. Like `createdAt`, it
is stored in the JSON document but deliberately left out of the search schema, so
it costs no index change and no re-index of existing data — it rides along and
comes back on the query row.

This is the pointer half of small-to-big retrieval: rank at memory granularity,
where BM25 discriminates well, then expand a match into the surrounding transcript
on demand. `ChatHistory` is the natural other half, since a memory's
`conversationId` is a `ChatHistory` `sessionId`.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
…d memory-provider.ts

Pure move, no behaviour change: the file held two independent integrations sitting
at different eve seams, and they shared no code — only the `Redis` and telemetry
imports.

  eve-memory.ts        barrel: the "two seams, which to pick" overview + re-exports
  memory-documents.ts  redisDocuments / RedisMemoryDocumentBackend
  memory-provider.ts   redisMemory

`eve-memory.ts` stays the tsup entry for the `./memory` subpath, so the published
export map and every consumer import are unchanged, and `dist/memory.js` exports
the same symbols. Verified by diffing the declared symbols across the split (none
lost, none added) and by the existing suite.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
…add conversations

`./memory` has never been published (@upstash/agentkit-eve@0.8.0 exports only `.`
and `./sandbox`), so none of this is breaking for a released consumer.

Automatic capture is now OFF by default. Captured utterances and curated facts
share one BM25 ranking and the utterances win: recall builds its query from the
user's current message, so a stored "What do you remember?" scores near-perfectly
against the next "What do you remember?". Measured against a live index — the
captured question scored 50.9, while "User likes cucumber." (saved deliberately
through save_memory) was cut from the top 5 entirely. Asking the agent what it
remembers is what degraded what it remembered.

`capture: boolean` and `extract` collapse into one `autoCapture` union —
false | true/"fromUser" | "fromModel" | "all" | an extractor fn — which also
removes the illegal state `capture: false` alongside an `extract` that silently
never ran. "fromModel"/"all" are worse than "fromUser" (the assistant's text is
derived from the recalled block, so the agent re-memorizes its own restatements)
and their JSDoc says so.

The remaining renames make each flat field say which phase it belongs to:
  maxCharacters      -> maxRecallCharacters   (the recalled block)
  maxEntryCharacters -> maxMemoryCharacters   (one stored memory)
  query              -> buildRecallQuery
  tools              -> memoryTools
  defaultExtract     -> defaultExtractMemories

New `conversations` option (default false) is small-to-big retrieval: it stores
each turn's transcript through core ChatHistory keyed by the eve session id, stamps
that id on every memory captured or saved in the turn, tags recalled memories
`conversation=<id>`, and contributes a `read_conversation` tool. Memories stay
ranked individually — what BM25 is good at — and the model expands a match into the
surrounding exchange on demand, so a remembered question can lead to the answer
that followed it without transcripts being injected into every prompt. The recalled
block is filtered out before storing, or recall output would round-trip into the
transcript recall later expands. The pointer is not a snapshot: the transcript keeps
growing after the memory is written.

`save_memory` now waits for indexing like capture already did. Upstash Search
indexes asynchronously with lag in the tens of seconds, so without it a model that
saves a fact and is asked about it next turn recalls nothing — which reads as the
save being lost. `waitForIndexing: false` opts out.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
…autoCapture is off

The eval asserted automatic capture ("Nothing calls a tool to save it"), which no
longer happens by default. Rather than turning autoCapture on in the demo — the
setting that makes an agent look amnesiac in interactive use — the mock model gains
a second trigger so each slot is exercised through its own save tool:

  "REMEMBER: <fact>" -> profile__save_memory  (eve's file memory, our Redis storage)
  "NOTE: <fact>"     -> recall__save_memory   (our MemoryProvider)

Recall itself is still asserted as automatic: eve runs the provider's turn.started
handler and injects the ranked block before the model sees anything, and the mock
echoes what arrived in its prompt. Automatic capture keeps its own coverage in
packages/eve/src/eve-memory.test.ts.

The demo slot also turns on `conversations`, so `recall__read_conversation` is
wired up in a real agent. Eval passes 10/10 gates against real Redis.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
@vercel/next pins `outputFileTracingRoot` to the app directory. In a workspace,
`next` and `@upstash/agentkit-eve` under examples/eve-demo/node_modules are symlinks
into the repo-root .pnpm store, which that root excludes — so the build failed with
"We couldn't find the Next.js package (next/package.json) from the project
directory". Both `outputFileTracingRoot` and `turbopack.root` now point at the
monorepo root; Next requires them to be equal.

With this, `vercel build` + `vercel deploy --prebuilt` from the repo root works
without publishing any workspace package. It needs the project linked at the root
with rootDirectory=examples/eve-demo, and `vercel pull` re-nulls `framework` and
`rootDirectory` in .vercel/project.json, so re-apply them after a pull.

Also gitignore `.vercel`, which was untracked at the repo root — `vercel pull`
writes project secrets into it.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
Pure move plus one clarifying rename; no behaviour change and no public API change.

  src/memory/index.ts      barrel + "two seams, which to pick" (the ./memory tsup entry)
  src/memory/documents.ts  redisDocuments / RedisMemoryDocumentBackend
  src/memory/provider.ts   redisMemory
  src/memory/memory.test.ts

`src/memory.ts` — the package-root tool factories `defineMemoryRecallTool` /
`defineMemorySaveTool` — is renamed to `src/memory-tools.ts`. It would still have
resolved (`./memory.js` prefers the file over the directory), but a `memory.ts`
sitting beside a `memory/` is a trap for the next reader, and the two are different
features: tools you drop into agent/tools/*.ts versus the memory-slot integrations.

dist/memory.js and dist/index.js export exactly the same symbols as before.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
…elMessage

The recall/capture helpers took `readonly unknown[]` and cast their way to
`role`/`content` on every access, which meant nothing was checked and a shape
change in eve would have surfaced as silently empty text rather than a type error.

They now use `ContextMessage = MemoryOperationContext["messages"][number]` — the AI
SDK `ModelMessage`, derived from eve's own context type rather than imported from
`ai`, which is only a devDependency here. `messageText` narrows the content parts
through their real discriminated union instead of a hand-rolled predicate, and
`textsWithRole` takes `ContextMessage["role"]` rather than `string`, so a typo like
"assistent" is now a compile error.

The provider tool map is likewise built as `Record<string, MemoryToolSet[string]>`,
so it is checked as it is assembled and the `as unknown as MemoryToolSet` on the
return is gone. The per-tool `as Parameters<typeof defineTool>[0]` casts stay: eve
types a provider tool's `execute` input as `never`, which no concrete input
satisfies.

No `unknown` left in provider.ts. The one in documents.ts is deliberate and now
says so — `@upstash/redis` auto-deserializes replies, so an `HMGET` field really
can come back as a number or object, and the `typeof` guards are the recovery.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
…vider

`./memory` has never been published (@upstash/agentkit-eve@0.8.0 exports only `.`
and `./sandbox`), so nothing here breaks a released consumer.

`redisDocuments()` earns its place on a test the provider did not pass: it is the
only way to get the capability. eve's `fileMemory()` resolves storage to an
in-process Map under `eve dev`, Vercel Blob on Vercel, and errors everywhere else,
and closing that gap needed the hard part — compare-and-swap over a stateless REST
API with no WATCH/MULTI, the content marker for auto-deserialization, and the
re-read guard for the sync-token lag. One job, finished, unchanged all week.

`redisMemory()` was a good implementation of a commodity. Two things decided it:

- Its API moved three times before release — capture default, six renames,
  conversations. That churn is what this repo's naming history is a museum of, and
  nothing was published yet, so holding costs nothing while shipping locks it.
- Once autoCapture had to default off (captured utterances outrank curated facts in
  a shared BM25 ranking — measured: a captured "What do you remember?" scored 50.9
  while a deliberately saved fact was cut from the top 5), its differentiator
  narrowed to "the store can exceed eve's 64 KiB / 4,000-char ceiling". Real, but
  much narrower than the docs claimed, and everything else it offered is already
  covered by defineMemoryRecallTool/defineMemorySaveTool, ai-sdk createMemoryTools
  and the extension's recall_memory/save_memory.

Also reverts the `conversationId` field on core AgentMemory: it existed only to
point a memory at a ChatHistory transcript for the provider's `conversations`
feature, and shipping an optional public field with no consumer is the same
unsettled-surface problem.

The provider and its ~25 tests stay in git history; CLAUDE.md records where to find
them and the one case worth resurrecting them for.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
…ctor form

Reinstates the provider removed in 7215be6 — ranked BM25 recall, capture,
`conversations`/`read_conversation`, and the `conversationId` passthrough on core
`AgentMemory` — with two deliberate narrowings, and `autoCapture` defaulting to on.

The backend alone only solves storage. `fileMemory()` recall replays one document
whole, so a slot backed by `redisDocuments()` cannot retrieve by relevance at all;
searchable memory was only reachable through the standalone tools, which the model
has to remember to call. Automatic ranked recall at `turn.started` is the thing
this package is for, and it lived here.

Narrowings:

- **`memoryTools` is gone.** `save_memory`/`forget_memory` are always contributed —
  a memory slot with no way to save or forget is a strange thing to declare, and
  the flag only existed because the tools and the transcript reader were once gated
  together.
- **`autoCapture` no longer takes a function.** The union is
  `true`/"fromUser" | "fromModel" | "all" | false, and `defaultExtractMemories` is
  now internal. Custom extraction was the least-used and most open-ended part of
  the surface; a caller who wants distilled facts can call `save_memory` with them.

`autoCapture` now defaults to `true`. The measured hazard is unchanged and stays
documented on the option, in the changeset and in CLAUDE.md: captured utterances and
curated facts share one BM25 ranking, and a stored "What do you remember?" scored
50.9 against the next one while a deliberately saved fact fell out of the top 5.
`autoCapture: false` is the model-curated escape hatch.

141 tests, both demo builds, and the demo's mocked-model eval (10/10 gates against
real Redis) all pass.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
…ed block holds

The hooks were only ever named in passing — "recall at turn.started /
compaction.completed, capture at turn.completed / compaction.requested" — as a bare
pairing, six times across provider.ts and once each in CLAUDE.md and the changeset.
The user-facing README did not mention them at all, so nothing said what happens at
each point or why the pairing is what it is.

Adds a lifecycle table covering both integrations, plus the two consequences that
are not guessable: capture runs after the response is delivered, which is what makes
blocking on waitIndexing() free; and recall runs a second time at
compaction.completed so memory is re-injected against the new checkpoint instead of
being folded into the summary. Also records that recall is cached per operationId
because eve treats that id as an idempotency key and rejects a differing replay.

Also documents what a recalled block can contain, including the gap: three sources
land in one list — save_memory facts, the caller's turn text, the assistant's reply
— and nothing distinguishes them. A record is {text, userId, createdAt,
conversationId?} with no source field, and both write paths share the
stableHash(text) id, so identical text collapses onto one record whichever way it
arrived. `autoCapture: false` is the only way today to guarantee every memory was
deliberately saved. The conversation= tag is present only for records written while
`conversations` was enabled; turning it on later does not backfill.

README carries the full version, the memory/index.ts barrel a condensed one,
formatRecall's JSDoc the block shape, and CLAUDE.md both plus the note that adding a
`source` field would need an indexed schema change (unlike conversationId, which
rides along unindexed).

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
`scope: (ctx) => ctx.session.auth.current?.principalId ?? ctx.session.id` fails
open: when no principal resolves it silently degrades to a per-session partition
instead of refusing. `byPrincipal` fails closed — it returns null for
anonymous/runtime callers, which disables the slot.

This does not collapse the alice/bob dropdown, because `demoUserAuth` runs before
`localDev()` in the channel's auth walk, so the UI's `x-user-id` header still
supplies the principal and each user keeps a separate partition. The eve TUI sends
no header and lands on the shared `local-dev` principal, which is what it did
before.

The comment on each slot now says outright that the header is demo-only and is not
a tenant boundary — anyone can set it — since these two files are what a reader
copies.

Changing the scope changes the partition key, so memories written under the old
scope are stranded rather than migrated. Eval still passes 10/10 gates.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
…memory with its source

Core `AgentMemory` is now generic — `AgentMemory<TMetadata>` — and `add()` takes a
`metadata` object that `recall()` returns on each hit. It replaces the single-purpose
`conversationId` field: one extensible passthrough instead of a growing list of
special cases. Like `createdAt` it is stored but left out of the search schema, so it
costs no index change and no re-index; the price is that it cannot be filtered or
searched on, which the JSDoc now says outright.

`redisMemory()` uses it to close the provenance gap. Every write stamps a source:

  "agent"        -> the model chose to remember it, via save_memory
  "userMessage"  -> captured from the caller's turn text
  "agentMessage" -> captured from the assistant's reply

and recall renders it per line, so the block now reads

  a1b2c3d4e5f6: The user prefers dark mode (you saved this, conversation=wrun_01ABC)
  9f8e7d6c5b4a: I ride a Brompton (the user said this)

with the preamble telling the model that a saved fact was chosen deliberately while
a captured turn may be off-hand. All three used to land in one ranked list with
nothing to tell them apart, which was documented as a limitation two commits ago;
this is that limitation fixed.

Extractors now return {text, source} rather than bare strings, so "all" tags each
half of a turn correctly instead of guessing from position.

Records with no metadata — written before this, or by the standalone memory tools
that share the same store — get no note rather than a guessed one, and there is a
test pinning that.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
…es the conversation tag

The example showed `conversation=` only on the saved-fact line, implying the tag is
tied to how a memory was written. It is not: capture stamps the id on every record it
writes that turn, and save_memory stamps it too, so with `conversations` enabled all
three sources carry it. The only records without one are those written before the
setting was turned on.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
…ult both on

Adds `<slot>__search_memory`. Automatic recall only ever surfaces what matches the
*current* message, so until now the model had no way to look something up after the
conversation changed topic — it could save and forget, but not search. Fuzzy match
over the memory text, `userId` pinned to the locked scope like every other tool,
capped at 25 results.

Renames the two options that decide what a slot does, from what the code does to
what the caller gets:

  autoCapture   -> rememberMessages
  conversations -> rememberSessions

"Session" is eve's own noun, not a synonym invented here: its docs use it 1011
times against 84 for "conversation", the id being stored is literally
`context.session.id`, and core ChatHistory's field is already `sessionId`.
`@supermemory/eve` independently named its equivalent tool `read_session`. So the
rename runs all the way through — `read_conversation` -> `read_session`, the
metadata field `conversationId` -> `sessionId`, and the recalled-block tag
`conversation=<id>` -> `session=<id>`.

Both options moved directly below `redis`, since everything else is tuning, and
both now default to on. `true` for `rememberMessages` means "all" — both halves of
a settled turn rather than the caller's text alone. The measured ranking hazard is
unchanged and still documented on the option: captured turns and saved facts share
one BM25 ranking, a captured question scored 50.9 against the next one, and
capturing the assistant's reply compounds it because the reply is derived from the
recalled block. `search_memory` and the per-record `source` label are what make that
liveable — the model can see which memories it chose and go looking when ranking
buries one.

Removes `buildRecallQuery`: the recall query is always the turn's user text.

Every optional config field now carries a JSDoc `@default` tag.

143 tests, both demo builds and the demo eval (10/10 gates against real Redis) pass.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
…lapsed sections

The Memory slots section had grown two long inline subsections — the four-hook
lifecycle table and the anatomy of a recalled block — ahead of the Options block,
so the page led with reference material before a reader had decided which
integration they wanted.

Both are now <details> like every other reference block in this README (memory
tools, search tools, rate limiting, sandbox), leaving the section itself as the two
snippets, the comparison table and the choice between them. No content changed.

Claude-Session: https://claude.ai/code/session_017VhM6aQAgDMH7wQqY5vLU9
…atch fallback

`metadataSchema` takes Upstash Search field builders whose values are supplied per
record as `metadata` and can then be filtered on in `recall({filter})`, plus new
`list({filter})` and `count({filter})`. Metadata is stored top-level, because Redis
Search indexes JSON by path and a nested object would not be filterable.

Omit `metadataSchema` and the store is exactly what it was — same two indexed
fields, same index, same keyspace, no re-index. That is what keeps this additive for
`ai-sdk`, `eve/memory-tools` and the extension runtime, which all share
`agentkit:memory`.

An extended store must use its own `prefix`, and the reason is verified rather than
stylistic: a document written without a `deleted` field is returned by
`{userId: {$eq: …}}` and by nothing that also filters `deleted: {$eq: false}`, and
Upstash Search rejects `$ne` outright. Extending the shared schema in place would
have made every record written by published 0.6.0 permanently unreachable — still in
Redis, never returned, no error.

Breaking: `recall()` no longer falls back to "everything for the user" when a query
matches nothing; it returns nothing. The fallback made a miss indistinguishable from
a hit, so a model reported unrelated memories as results — black-box testing caught
an agent claiming "I do not see that in the stored entries" from an unfiltered dump
it took for a filtered one. Omitting the query is still how you ask for the whole
set, and every memory-tool caller is affected.

Claude-Session: https://claude.ai/code/session_01J4MAFwbxLvA11NFHGmjXF2
The slot kept facts in AgentMemory and transcripts in ChatHistory, and nothing
reconciled them. Black-box testing against examples/eve-demo showed what that cost:

- deletion could not be honest. forget_memory deletes one memory key and nothing
  ever deleted from the transcript, so 5 of 29 records still contained a value the
  agent reported it had permanently erased.
- captured turns buried curated facts. Recall queries with the caller's current
  message, so a stored "What do you remember?" scored 50.9 against the next
  identical question while "User likes cucumber." was cut from the top 5.
- the transcript half was unreachable. Across 32 conversations where read_session
  existed, was advertised and had transcripts in Redis, the model called it zero
  times — once answering "MY SIDE NOT AVAILABLE" with the answer one tool call away.

Everything now lives in one keyspace of the slot's own, `agentkit:memorySlot`, with
sessionId/source/deleted indexed and sequence/subIndex along for ordering. Its own
keyspace is required, not tidiness: a schema with extra fields must not cover the
shared `agentkit:memory` prefix, whose existing records lack them and would become
unreachable.

- recall injects source:"agent" only, so captured turns share the store but not the
  ranking. The block ends with a live count pointing at search_memory, because the
  model does not use a tool it is merely offered.
- forget_memory redacts rather than deletes: text erased, deleted set, invisible to
  every read except read_session, which renders [redacted] so a reader cannot mistake
  removal for "never said".
- read_session replays a session sorted (sequence, sourceRank, subIndex), where
  source doubles as the intra-turn ordinal: the caller speaks, the model saves, it
  answers.

Removes rememberSessions (read_session is always contributed) and the
compaction.requested capture — messages are stored as they happen, so the summarizer
takes nothing with it, and it was the only context where sequence could be null.

Claude-Session: https://claude.ai/code/session_01J4MAFwbxLvA11NFHGmjXF2
…_memory when replies are stored

Black-box retesting the rebuilt provider over 18 conversations found the one thing
it still could not do honestly: delete.

The curated fact was correctly redacted. But the phrase the caller asked to erase
survived in three other records, and every one was an `agentMessage` — the
assistant's own replies *about* the deletion. Confirming an erasure records the
erased text, so deleting writes a fresh copy of what it deleted, and deleting more
would write more. A fourth survivor was the caller's own search query.

Two changes follow from that.

`rememberMessages` now defaults to `true` meaning "fromUser" rather than "all". In
the same run the assistant's replies were 18 of 41 stored records — half the store,
and the entire source of the leak. They are also derived from the recalled block, so
capturing them re-memorizes the agent's own restatements.

`"all"` and `"fromModel"` no longer contribute `forget_memory` at all. Those modes
store replies, so deletion cannot be honoured, and a tool answering "permanently
deleted every stored item that mentioned it" is worse than no tool — a caller
reasonably believes it. `search_memory` and `read_session` still reach everything;
only the claim to remove goes away. Gating covers "fromModel" as well as "all"
because it has the identical property.

The reasoning lives on the `rememberMessages` JSDoc, in the README, and in CLAUDE.md
with a note not to restore the tool for consistency: it was removed because it
cannot tell the truth there.

Claude-Session: https://claude.ai/code/session_01J4MAFwbxLvA11NFHGmjXF2
Five tests over two suites. The first three are the feature: metadata round-trips
through add and recall with a non-string field intact, a ranked recall narrows by a
metadata field, and list()/count() read by filter alone.

The other two are regression guards for the reasoning behind the API, which is the
part that would be expensive to rediscover:

- A record lacking a declared field is returned by `{userId}` alone and by nothing
  that also filters on that field. That is why an extended schema must not cover a
  keyspace holding records written without it — there is no filter-level workaround,
  since Upstash Search has no `$ne`. Without this test the prefix rule reads like
  style advice.
- An unextended store still reads records written before `metadataSchema` existed,
  stores no extra fields, and leaves `metadata` undefined rather than `{}`. That is
  the non-breaking claim, asserted rather than asserted-in-prose.

README documents it behind a details block: the schema, filtering, that values are
stored top-level because Redis Search indexes JSON by path, and the own-prefix
warning stated as data loss rather than a preference. `list`/`count` added to the
method list.

Claude-Session: https://claude.ai/code/session_01J4MAFwbxLvA11NFHGmjXF2
@CahidArda
CahidArda force-pushed the feat/eve-redis-memory branch from ac75b1a to dd54011 Compare September 3, 2026 09:48
CI went red on `AgentMemory without metadataSchema is unchanged` with an empty
result set where two just-written records were expected. Each suite here mints a
`uniquePrefix`, so every run starts with an index that does not exist yet: the
writes land first, `waitIndexing()` on a missing index is a silent no-op, and the
first `recall()` is what provisions it reactively. That read then asserted
immediately against an index whose backfill had not caught up, with no retry.

Apply the ordering the two already-fixed suites use (chat-history, eve
search-tools): provision in `beforeAll` via a throwaway `count()` — the
`{count:-1}` sentinel makes the reactive wrapper create the index and wait — then
seed, `waitIndexing()`, and read through a bounded `pollUntil` for residual lag.

The miss assertion in "returns nothing when a query matches nothing" now confirms
the record is visible *before* asserting the miss, so it can no longer pass
because the doc simply had not been indexed yet.

Test-only; no package behaviour changes.

Claude-Session: https://claude.ai/code/session_01J4MAFwbxLvA11NFHGmjXF2
…he reads

Same fix as the previous commit, applied to the remaining suites CLAUDE.md flags
as still seeding before their index reliably exists. The ai-sdk search-tools
suite is what went red on the last CI run ("count tool counts matching
documents": expected 1 to be >= 2) — it counted while the index had caught up
with only one of the two docs the previous test seeded, and asserted once with no
retry.

Each suite now creates its index in `beforeAll` via a throwaway read (a missing
index answers `count` with `{count:-1}` and `query` with `null`, either of which
makes the reactive wrapper create it and retry), then seeds, waits, and reads
through a bounded `pollUntil`.

`reactive-index.test.ts` is deliberately left alone: it asserts on `createIndex`
call counts against empty indexes, so it has no read-after-write assertion to
race, and provisioning up front would defeat what it tests.

Test-only; no package behaviour changes.

Claude-Session: https://claude.ai/code/session_01J4MAFwbxLvA11NFHGmjXF2
`metadataSchema` was typed `Record<string, unknown>` and the metadata type was a
separate, hand-written type parameter, so nothing tied them together: a schema
could declare `deleted: s.boolean()` while the metadata type called it a string,
`metadata` could carry keys the schema never indexed, and `filter` was
`Record<string, unknown>` — a typo or a wrong operand type compiled fine and
matched nothing at query time.

`AgentMemory`'s first type parameter is now the schema, inferred from the
argument, and `metadata`, `recall/list/count`'s `filter` and the returned records
are all derived from it. TypeScript has no partial type-argument inference, so
this is the only arrangement that can actually check the two against each other:
had the metadata type stayed the inferred-from-nothing first parameter, a schema
passed as a value could never be compared to it.

Where the derived type is too wide — a `s.string()` field holding a known union —
the metadata type can still be given as a second argument, constrained to
`MetadataOf<TSchema>` so it cannot contradict the schema. That is what the eve
memory provider uses to keep `source: MemorySource` instead of `string`.

The builder classes are not exported by `@upstash/redis`, so the built field type
is recovered structurally (the single zero-arg method returning a `{type: …}`
object); only the field-type-to-value mapping is restated from the library.

`memory.types.test.ts` pins all of it with `@ts-expect-error` markers, which fail
the build if an invalid usage becomes legal — reverting the two signatures turns
8 of them red.

BREAKING CHANGE: `AgentMemory<TMetadata>` is now `AgentMemory<TSchema>`. Callers
naming the metadata type explicitly either drop the argument and let it infer
from `metadataSchema`, or pass both as `AgentMemory<typeof schema, TMetadata>`.
Only released as part of the unshipped `metadataSchema` feature.

Claude-Session: https://claude.ai/code/session_01D3v2QieC7eC1EwhZk7KtKj
`recall`, `list` and `count` all funnel into the private `query()`, whose `filter`
was still `Record<string, unknown>` — the one seam the previous commit left
loose. It now takes `MetadataFilter<TSchema>` like its callers, so an internal
caller cannot pass a filter the schema does not describe either.

The two remaining `as InferFilterFromSchema<…>` assertions stay, and the comments
now say why rather than leaving it to be rediscovered: the index handle is typed
with the base schema while the index also covers the declared metadata fields.
Typing the handle with the full schema was tried and does not help — a value
cannot be checked against a filter type built on an unresolved generic, so the
cast only moves to the construction seam and the reads need one anyway.

Claude-Session: https://claude.ai/code/session_01D3v2QieC7eC1EwhZk7KtKj
@CahidArda
CahidArda requested a lite review from Copilot September 4, 2026 10:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There are verified correctness/documentation mismatches (and one functional deletion-path bug) that should be fixed before merging to avoid misleading users and incorrect runtime behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds first-class support for eve’s native memory slots backed by Upstash Redis via a new @upstash/agentkit-eve/memory subpath, and extends the core SDK AgentMemory to support typed, filterable indexed metadata (plus related test and demo coverage). This fits the codebase’s direction of keeping all persistence/search on Upstash Redis, while offering both “eve fileMemory storage” and “ranked recall provider” seams.

Changes:

  • Add @upstash/agentkit-eve/memory with redisDocuments() (CAS/Lua-backed MemoryDocumentBackend) and redisMemory() (full MemoryProvider on AgentMemory).
  • Extend AgentMemory with metadataSchema → typed metadata + typed filter, plus new list() and count(), and change recall() miss behavior (no fallback-to-all).
  • Add/strengthen live Redis tests, add an eve-demo mocked-model eval, and wire the eval into CI.
File summaries
File Description
README.md Mentions eve memory-slot support at the repo level.
packages/sdk/src/memory.types.test.ts Adds compile-time type-safety tests for metadataSchemametadata/filter.
packages/sdk/src/memory.ts Implements metadataSchema, typed filters, list()/count(), and recall miss behavior change.
packages/sdk/src/memory.test.ts Expands live Redis tests (provisioning + polling + metadataSchema coverage).
packages/sdk/src/index.ts Re-exports new AgentMemory metadata types.
packages/sdk/README.md Documents metadataSchema, list()/count(), and new recall miss semantics.
packages/eve/tsup.config.ts Adds build entry for the new ./memory subpath.
packages/eve/src/telemetry.test.ts Updates import path after memory tools refactor (memory-tools).
packages/eve/src/memory/provider.ts Introduces redisMemory() eve MemoryProvider over AgentMemory.
packages/eve/src/memory/index.ts Adds the @upstash/agentkit-eve/memory barrel and user-facing guidance.
packages/eve/src/memory/documents.ts Adds redisDocuments() MemoryDocumentBackend with Lua CAS + marker encoding.
packages/eve/src/memory-tools.ts Updates tool messaging/comments for new recall() semantics.
packages/eve/src/memory-tools.test.ts Makes live Redis tests more reliable (provision + polling).
packages/eve/src/index.ts Exports memory tools from memory-tools and documents the ./memory subpath.
packages/eve/README.md Documents eve memory slots and their Redis-backed implementations.
packages/eve/package.json Adds ./memory subpath export and updates package description.
packages/eve-extension/extension/tools/recall_memory.ts Updates comment to match new recall() miss behavior.
packages/ai-sdk/src/search-tools.test.ts Makes live Redis search-tools tests more reliable (provision + polling).
packages/ai-sdk/src/memory.test.ts Makes live Redis memory-tools tests more reliable (provision + polling).
examples/eve-demo/README.md Documents the new memory slots and adds mocked-model eval instructions.
examples/eve-demo/next.config.ts Fixes monorepo tracing configuration for Turbopack/Next in pnpm workspace.
examples/eve-demo/evals/memory.eval.ts Adds end-to-end eval for both slot integrations against real Redis.
examples/eve-demo/evals/evals.config.ts Adds eval config file for eve eval runner.
examples/eve-demo/agent/memory/recall.ts Adds the redisMemory()-backed slot in the demo.
examples/eve-demo/agent/memory/profile.ts Adds the fileMemory({ backend: redisDocuments() }) slot in the demo.
examples/eve-demo/agent/instructions.md Documents how the demo agent should use the two slots/tools.
examples/eve-demo/agent/agent.ts Adds AGENTKIT_MOCK_MODEL support via mockModel for evals.
docs/memory-redesign.md Captures the design rationale and observed behaviors behind the new approach.
CLAUDE.md Updates repo guide with the new eve memory-slot subpath details.
.gitignore Ignores .vercel.
.github/workflows/ci.yml Adds CI step for the new eve-demo mocked-model memory-slot eval.
.changeset/sdk-memory-metadata.md Release notes for SDK AgentMemory metadata changes + recall semantics change.
.changeset/eve-redis-memory-slots.md Release notes for the new eve memory-slot subpath.
Review details

Suppressed comments (1)

packages/eve/src/memory/provider.ts:125

  • RedisMemoryConfig.prefix JSDoc says the default is agentkit:memory and that slots share the tools’ index, but the provider actually defaults to agentkit:memorySlot specifically to avoid schema/keyspace mixing. Please align this comment with the actual default and rationale (separate keyspace + index).
   * Base key prefix for stored memories. Defaults to `agentkit:memory` — the same store
   * {@link defineMemorySaveTool} writes to, so slots and tools share one Redis Search index
   * (an Upstash database caps at 10). Memories are still isolated: the per-user key part is eve's
   * scope key, which no tool-based `userId` can collide with.
   */
  • Files reviewed: 33/34 changed files
  • Comments generated: 6
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/eve/src/memory/provider.ts Outdated
Comment thread .changeset/eve-redis-memory-slots.md Outdated
Comment thread examples/eve-demo/agent/agent.ts Outdated
Comment thread examples/eve-demo/agent/memory/recall.ts Outdated
Comment thread packages/eve/src/memory/index.ts
Comment thread packages/eve/src/memory/provider.ts Outdated
…s workaround

`@upstash/redis` sent its `upstash-sync-token` one request late through 1.38.0:
`HttpClient.request()` built the outgoing headers before copying the latest token
into them, so a read issued straight after a write could reach a replica that had
not caught up. `RedisMemoryDocumentBackend` worked around it by remembering the
scope keys it had written and re-reading (up to twice) before returning `null`
for one of them.

1.38.4 fixes the ordering upstream, so the workaround is gone: `read()` is a
plain `HMGET` again, and the FIFO memo, its bound and the write-side bookkeeping
go with it.

Verified rather than assumed. Stubbing `fetch` and reading the token off each
outgoing request, three calls send `[null, "", "tok-1"]` on 1.38.0 — every
request one token behind — and `["", "tok-1", "tok-2"]` on 1.38.4.

`packages/eve`'s `@upstash/redis` peer floor is raised `>=1.38.0` -> `>=1.38.4`,
because `redisDocuments()` now relies on the fix; leaving it open would let a
consumer reinstate the bug with no workaround left to catch it. Every dependency
and devDependency pin moves to `^1.38.4`. `pnpm dedupe` collapses the second copy
that `@upstash/core-analytics` (via `@upstash/ratelimit`) was holding at 1.38.0.

The scripted lagging-client regression test is removed with the behaviour it
pinned; its sibling now asserts the plain path — an absent document resolves in a
single round trip. The `pollUntil`s elsewhere are for search-index lag, a
different problem, and stay.

Claude-Session: https://claude.ai/code/session_01D3v2QieC7eC1EwhZk7KtKj
Vercel clarified the memory-provider contract in vercel/eve#2951 after we asked
about it. eve does the replay bookkeeping itself — it records a digest of the
accepted recall result and rejects a replay that differs — so a provider does not
need to persist recall results by `operationId` "unless its store can change
before a replay". That is why supermemory and `fileMemory()` do not cache.

We are that exception, so the cache stays. Recall is a live ranked query plus two
live counts over a store the same turn writes to: `save_memory` adds a
`source: "agent"` record, which is exactly what recall ranks, and the model can
call it mid-turn; `forget_memory` flips `deleted`; capture appends the turn's
messages; and a concurrent session on the same scope key can do any of it.
Replaying `turn.started` after any of those would produce a different block and
eve would reject the turn.

The old comment justified the cache as "a live ranked query is not naturally
stable", which reads like eve requires every provider to cache. It does not, and
that framing invites deleting the cache on the next read of this file. Comments
only — no behaviour change.

Claude-Session: https://claude.ai/code/session_01D3v2QieC7eC1EwhZk7KtKj
…slot docs

Addresses the Copilot review on #33. All six findings were real; the first is a
behaviour bug, the rest are documentation that still describes the pre-redesign
design.

**`forget_memory` could refuse to redact a record that exists.** It listed one
unranked page of live records (`limit: 50`) and filtered that page for the id, so
once a scope held more live memories than a page, forgetting anything outside it
returned `{redacted: false, reason: "no entry with that id"}` while the record
stayed recallable — the user asks to forget something and is told it was never
there. It now reads the key directly via a new `AgentMemory.get({userId, id})`,
which no page can hide and which also sees records the index has not caught up
with. Regression test seeds a full page *before* the target, so the target lands
outside it; reverting to the page scan turns it red ("expected false to be true").

`AgentMemory.get` is a real addition to the core SDK: a direct-key read was
missing, and looking a known id up through a bounded search was the only option.

Docs corrected to match the shipped code:
- the provider docstring and `prefix` JSDoc claimed the slot stores at
  `agentkit:memory` sharing `defineMemorySaveTool`'s index; it has owned
  `agentkit:memorySlot` and its own index since the schema gained indexed fields
- the lifecycle tables in `memory/index.ts` and the changeset advertised capture
  at `compaction.requested` and a `rememberSessions` option; only
  `turn.completed` is registered and that option no longer exists
- both eve-demo memory comments were wrong about `rememberMessages`, one saying
  capture is off by default and the other that `true` means `"all"`; it defaults
  to `true`, which means `"fromUser"`

Claude-Session: https://claude.ai/code/session_01D3v2QieC7eC1EwhZk7KtKj
They had grown into design documents — 137 lines for the eve entry alone, 213
across the three — carrying measurement numbers, black-box run counts, test
descriptions and implementation archaeology that belong in CLAUDE.md, the code
comments and the PR, not in a consumer's CHANGELOG.

The eve entry had also gone stale twice over: an implementation note still
described the `read()` read-your-writes workaround that the 1.38.4 bump deleted,
directly contradicting the paragraph at the bottom saying it was gone.

Rewritten to what a consumer needs: what the API is, what it requires (eve
≥0.45.2, `@upstash/redis` >=1.38.4), and the behaviours that would surprise
someone configuring it. Every retained default and option name re-checked against
the source. The sdk entry now also mentions `get()`, which it had never listed.

213 -> 109 lines; the eve entry 137 -> 45.
It does. `redis.multi()` posts to a dedicated `/multi-exec` endpoint and executes
atomically — measured against a live database, `multi().set().get().incr().exec()`
returns `["OK","a",1]`. The comments here asserted REST "is stateless and
therefore has no WATCH/MULTI", which is half wrong and was never verified.

The conclusion is unchanged: the compare-and-set still has to be a Lua `EVAL`.
The accurate reason is narrower. A transaction queues its commands and hands back
every result at `EXEC`, so nothing inside it can branch on a value it just read —
`multi().get(k).set(k,"b").exec()` returns `["a","OK"]` with the `set` already
done unconditionally. Conditioning a write on what was read is `WATCH`'s job, and
`WATCH` is the part REST genuinely lacks: the server answers
`ERR Command "WATCH" is not allowed in REST`, since watching spans requests and
REST keeps no session between them.

Corrected in the `documents.ts` module docstring, the comment above the `EVAL`
call, the concurrent-writers test, and CLAUDE.md — which now records the measured
behaviour of both, so the wrong version does not get written back.

Comments only; no behaviour change.

Claude-Session: https://claude.ai/code/session_01D3v2QieC7eC1EwhZk7KtKj
Last remaining copy of the corrected claim. `MULTI` is available over Upstash's
REST API via the /multi-exec endpoint; it is `WATCH` that is not, which is why
the conditional write is a Lua `EVAL`.

The previous sweep missed this one because the sentence wrapped across a line
break and the search was line-based.

Claude-Session: https://claude.ai/code/session_01D3v2QieC7eC1EwhZk7KtKj

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The redisMemory() capture logic currently dedupes by text only, which can drop distinct turn entries (e.g., user vs assistant) and make read_session transcripts incomplete.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file
  • Files reviewed: 40/42 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread packages/eve/src/memory/provider.ts Outdated
Comment thread CLAUDE.md Outdated
Second Copilot pass on #33; both findings were real.

Capture filtered its batch through a `Set` keyed on the normalized text alone.
Under `rememberMessages: "all"` both halves of a turn are captured, and when the
caller and the model say the same short thing — "thanks", "yes", "ok" — the
second was dropped, so `read_session` returned half the turn. Nothing else forced
that: `recordIdFor` already mixes in `source` and `subIndex`, so the two records
have distinct keys and both would have stored fine. The batch filter is now keyed
by source plus text, which still collapses a genuine repeat within one source.

It mattered more than a stray duplicate because this transcript is meant to be
gap-free: `forget_memory` redacts rather than deletes precisely so a reader never
sees a silent hole and re-derives what was removed. A capture that quietly
skipped an entry punched exactly that hole.

Regression test drives an echoed turn through `"all"` and asserts both sources
come back; reverting to the text-only key fails it with
`expected [ 'userMessage' ] to deeply equal [ 'userMessage', 'agentMessage' ]`.

CLAUDE.md still described capture at `turn.completed`/`compaction.requested` in
three places, and the same bullet still described the pre-redesign record key and
index name. Corrected, with a note not to re-add the hook from an older reading.

Claude-Session: https://claude.ai/code/session_01D3v2QieC7eC1EwhZk7KtKj
@CahidArda
CahidArda merged commit 0117c2e into main Sep 4, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants